fix(tip-1034): sync escrow implementation with spec by legion2002 · Pull Request #3837 · tempoxyz/tempo

legion2002 · 2026-05-06T14:14:55Z

Stacks on #3704.\n\nSyncs the Rust TIP-1034 escrow precompile and ABI with the updated spec/reference implementation: expiring-nonce channel IDs, operator settlement, terminal state deletion, and same-transaction reopen protection.\n\nAdds focused coverage for the escrow spec drift. Payment-lane classifier changes are intentionally left out and will land in a separate PR.

github-actions · 2026-05-06T14:15:57Z

⚠️ Changelog not found.

A changelog entry is required before merging. We've generated a suggested changelog based on your changes:

Preview

---
tempo-alloy: minor
tempo-contracts: major
tempo-precompiles: major
---

Added `operator` and `expiringNonceHash` fields to TIP20 channel escrow descriptors, replaced the finalized close sentinel with explicit state deletion, and seeded a transient channel-open context hash from the enclosing transaction so `open()` is replay-protected. Allowed the operator (in addition to the payee) to call `settle`, and updated RPC simulation to populate the new context hash.

Add changelog to commit this to your branch.

tempoxyz-cyclops-bot

👁️ Cyclops Review

PR #3837 updates the TIP-1034 channel-escrow precompile to match the spec. The escrow code is gated behind T5 (T2 is currently active), so all impacts described below are latent until T5 activates.

Head drift note: consolidation was scheduled against e4d0fc0b313b; current head is b17b0142…. Both findings re-checked against the new head and still apply (the field was renamed expiring_nonce_hash → channel_open_context_hash and moved to top-level TempoTxEnv; the RPC sentinel changed from Some(B256::ZERO) to None, but the handler skips seeding the precompile slot when the hash is None, so open() still reverts in simulation).

⚠️ [ISSUE] Channel-escrow lifecycle calls are still classified as non-payment (file outside this diff)

Severity: Medium (latent until T5; spec-vs-impl divergence today)

Files (not in this PR's diff):

crates/primitives/src/transaction/envelope.rs:174-225 (is_payment_v1, is_payment_v2)
crates/primitives/src/transaction/envelope.rs:475-485 (is_tip20_call, is_tip20_payment)
crates/transaction-pool/src/transaction.rs:61-65 (cached is_payment_v2 flag)
crates/payload/builder/src/lib.rs:480-493 (non-payment general_gas_limit skip)
crates/evm/src/block.rs:410-414 (consensus non-payment gas-left handling)

Summary: TIP-1034 §"Payment-Lane Integration (Mandatory)" (tips/tip-1034.md:259-290, 313-314) requires escrow lifecycle calls (open, settle, topUp, close, requestClose, withdraw) to be recognized by is_payment_v1 (consensus) and is_payment_v2 (pool/builder) via an is_channel_escrow_payment(to, input) helper. Neither classifier was updated; both still match only the TIP-20 0x20c0… prefix. Because TIP20_CHANNEL_ESCROW_ADDRESS = 0x4D5050… does not satisfy that prefix, every syntactically valid escrow call is classified as non-payment, and the misclassification is consumed by the pool's cached is_payment flag, the payload builder's non-payment gas-limit gate, and consensus block sectioning.

Impact (post-T5): During the 15-minute close-grace window, a payee's settle()/close() is no longer protected by payment-lane capacity reservation; congestion in the non-payment lane lets the payer's withdraw() race past the grace boundary. This breaks the close-grace economic invariant TIP-1034 explicitly relies on.

Recommended Fix: Implement is_channel_escrow_payment(to, input) (selector + ABI-length + Tempo-signature-shape checks) and wire it into both is_payment_v1() and is_payment_v2(). Preserve the existing side-effect exclusions (authorization_list, tempo_authorization_list, key_authorization, empty AA call list). Add regression tests across Legacy / EIP-2930 / EIP-1559 / EIP-7702 / AA envelopes plus pool and builder integration. The PR's commit history shows this was deliberately split off as a follow-up, but the spec invariant is asserted as MUST in the merged TIP text.

Reviewer Callouts

⚡ Sentinel-collision pattern in RPC simulation envs: crates/alloy/src/rpc/reth_compat.rs still uses several "zero is harmless" placeholders (signature_hash: B256::ZERO, tx_hash: B256::ZERO, expiring_nonce_hash: Some(B256::ZERO) inside TempoBatchCallEnv). The first finding shows the invariant is fragile; any new precompile consumer of these fields must explicitly handle zero. Worth auditing every future use of tempo_tx_env.{tx_hash, signature_hash, expiring_nonce_hash} for the same trap.
⚡ Transient layout WARNING is unenforced: the // WARNING: comment on crates/precompiles/src/tip20_channel_escrow/mod.rs:62-64 says transient slots must remain after persistent slots until the contract macro supports independent layouts. There is no compile-time check; consider splitting the transient fields into a separate struct now.
⚡ Voucher signature gas charged via internal Rust call, not via a TIP-1020 precompile CALL: validate_voucher constructs a SignatureVerifier::new() and calls .recover(...) directly, deducting SECP256K1_VERIFY_GAS / P256_VERIFY_GAS / WEBAUTHN_VERIFY_GAS through the shared StorageCtx. Cryptographic rules are identical, but a strict reading of TIP-1034 rule 1 ("verify via the TIP-1020 precompile") might flag the gas divergence (the calldata cost of an actual external CALL is not paid).
⚡ Per-token TIP-403 dependency: every outbound leg (settle/close/withdraw) calls system_transfer_from(self.address, payer/payee, …), which goes through validate_transfer → ensure_transfer_authorized(escrow, recipient). Tokens whose TIP-403 policy does not whitelist TIP20_CHANNEL_ESCROW_ADDRESS as a sender will not be usable with the escrow. Operators integrating new TIP-20 tokens with the escrow must remember to whitelist the escrow address as both sender and recipient.
⚡ Access keys + open: an access key with a broad call scope that includes TIP20_CHANNEL_ESCROW_ADDRESS can spend up to the master account's TIP-20 spending limit by opening a channel where a colluding address is the payee/authorizedSigner. Spending limits are correctly applied at open; worth a user-facing docs note when granting access-key call scopes.
⚡ test_dispatch_rejects_static_mutation in tip20_channel_escrow/mod.rs does not actually exercise a static call; it only checks that escrow.call(...) succeeds in a non-static context. Static rejection is enforced by mutate/mutate_void and tested elsewhere, but the test name is misleading.
⚡ top_up with additionalDeposit = 0 is legal per spec — it lets the payer cancel a pending close request without adding any funds. Not a security issue, but indexers should be aware these zero-amount events are intentional.

tempoxyz-cyclops-bot · 2026-05-07T12:25:37Z


+    fn enclosing_channel_open_context_hash(&self) -> Result<B256> {
+        let hash = self.channel_open_context_hash.t_read()?;
+        if hash.is_zero() {


🚨 [SECURITY] RPC simulation of TIP20ChannelEscrow.open() always reverts with ExpiringNonceHashNotSet

Severity: Medium (latent until T5)

The RPC pipeline (eth_call / eth_estimateGas) builds a TempoTxEnv via TempoTransactionRequest::try_into_tx_env, which hard-codes channel_open_context_hash: None (crates/alloy/src/rpc/reth_compat.rs:135). The handler's seed_precompile_tx_context only forwards the hash to TIP20ChannelEscrow::set_channel_open_context_hash when it is Some(_) (crates/revm/src/handler.rs:418-421), so the precompile's transient slot is never written and reads as zero. enclosing_channel_open_context_hash here treats the zero TLOAD as "unset" and returns expiring_nonce_hash_not_set, so every simulated open() reverts even though the same call would succeed on-chain (where crates/revm/src/tx.rs populates a non-zero keccak256(encode_for_signing || sender) for every real tx type).

Impact: eth_estimateGas for open() either errors or binary-searches on a path that never produces a successful open; eth_call previewing the returned bytes32 channelId always errors with ExpiringNonceHashNotSet; wallets/SDKs that simulate before submitting will refuse otherwise-valid open txs once T5 is active.

Recommended Fix: Either (a) compute a deterministic non-zero channel_open_context_hash for the simulation env in reth_compat.rs (mirroring the real formula or substituting any non-zero placeholder, then documenting that any non-zero value is acceptable), or (b) distinguish "set" from "not set" in the precompile via an explicit transient flag rather than the hash.is_zero() sentinel. Option (a) is the smaller and more direct fix.

addressed in e11c01f

Amp-Thread-ID: https://ampcode.com/threads/T-019dfd57-2d44-729b-a226-99e1cc5cdf05

github-actions · 2026-05-07T12:43:06Z

📊 Tempo Precompiles Coverage

precompiles

Coverage: 5634/8218 lines (68.56%)

File details

File	Lines	Coverage
`src/account_keychain/dispatch.rs`	30/68	44.12%
`src/account_keychain/mod.rs`	274/736	37.23%
`src/address_registry/dispatch.rs`	31/33	93.94%
`src/address_registry/mod.rs`	50/56	89.29%
`src/error.rs`	39/116	33.62%
`src/ip_validation.rs`	10/10	100.00%
`src/lib.rs`	181/221	81.90%
`src/nonce/dispatch.rs`	9/10	90.00%
`src/nonce/mod.rs`	46/61	75.41%
`src/signature_verifier/dispatch.rs`	19/20	95.00%
`src/signature_verifier/mod.rs`	13/17	76.47%
`src/stablecoin_dex/dispatch.rs`	92/93	98.92%
`src/stablecoin_dex/mod.rs`	876/918	95.42%
`src/stablecoin_dex/order.rs`	110/161	68.32%
`src/stablecoin_dex/orderbook.rs`	157/216	72.69%
`src/storage/evm.rs`	192/221	86.88%
`src/storage/hashmap.rs`	0/158	0.00%
`src/storage/mod.rs`	27/27	100.00%
`src/storage/packing.rs`	68/93	73.12%
`src/storage/thread_local.rs`	165/227	72.69%
`src/storage/types/array.rs`	0/72	0.00%
`src/storage/types/bytes_like.rs`	79/162	48.77%
`src/storage/types/mapping.rs`	27/48	56.25%
`src/storage/types/mod.rs`	67/91	73.63%
`src/storage/types/primitives.rs`	21/24	87.50%
`src/storage/types/set.rs`	28/192	14.58%
`src/storage/types/slot.rs`	55/81	67.90%
`src/storage/types/vec.rs`	101/246	41.06%
`src/tip20/dispatch.rs`	149/165	90.30%
`src/tip20/mod.rs`	590/693	85.14%
`src/tip20/rewards.rs`	238/252	94.44%
`src/tip20/roles.rs`	107/110	97.27%
`src/tip20_channel_escrow/dispatch.rs`	0/51	0.00%
`src/tip20_channel_escrow/mod.rs`	3/453	0.66%
`src/tip20_factory/dispatch.rs`	17/18	94.44%
`src/tip20_factory/mod.rs`	105/125	84.00%
`src/tip403_registry/dispatch.rs`	55/56	98.21%
`src/tip403_registry/mod.rs`	334/371	90.03%
`src/tip_fee_manager/amm.rs`	285/364	78.30%
`src/tip_fee_manager/dispatch.rs`	81/83	97.59%
`src/tip_fee_manager/mod.rs`	71/136	52.21%
`src/validator_config/dispatch.rs`	38/52	73.08%
`src/validator_config/mod.rs`	171/227	75.33%
`src/validator_config_v2/dispatch.rs`	71/73	97.26%
`src/validator_config_v2/mod.rs`	552/611	90.34%

contracts

Coverage: 1/274 lines (0.36%)

File details

File	Lines	Coverage
`src/lib.rs`	1/1	100.00%
`src/precompiles/account_keychain.rs`	0/37	0.00%
`src/precompiles/address_registry.rs`	0/9	0.00%
`src/precompiles/nonce.rs`	0/18	0.00%
`src/precompiles/signature_verifier.rs`	0/3	0.00%
`src/precompiles/stablecoin_dex.rs`	0/21	0.00%
`src/precompiles/tip20.rs`	0/49	0.00%
`src/precompiles/tip20_channel_escrow.rs`	0/48	0.00%
`src/precompiles/tip20_factory.rs`	0/9	0.00%
`src/precompiles/tip403_registry.rs`	0/15	0.00%
`src/precompiles/tip_fee_manager.rs`	0/15	0.00%
`src/precompiles/validator_config.rs`	0/13	0.00%
`src/precompiles/validator_config_v2.rs`	0/36	0.00%

Total: 5635/8492 lines (66.36%)

📦 Download full HTML report

legion2002 requested review from 0xKitsune, 0xrusowsky, dankrad, danrobinson, fgimenez, klkvr, mattsse and rakita as code owners May 6, 2026 14:14

legion2002 force-pushed the tanishk/tip-1034-spec-sync branch 7 times, most recently from 1a2ffb4 to b17b014 Compare May 7, 2026 12:20

legion2002 requested a review from onbjerg as a code owner May 7, 2026 12:20

tempoxyz-cyclops-bot reviewed May 7, 2026

View reviewed changes

fix(tip-1034): sync escrow implementation with spec

d70f4b6

Amp-Thread-ID: https://ampcode.com/threads/T-019dfd57-2d44-729b-a226-99e1cc5cdf05

legion2002 force-pushed the tanishk/tip-1034-spec-sync branch from b17b014 to d70f4b6 Compare May 7, 2026 12:30

fix(rpc): seed channel open context in simulations

e11c01f

legion2002 merged commit 88e4dcb into tanishk/tip-1034-impl May 7, 2026
29 of 31 checks passed

legion2002 deleted the tanishk/tip-1034-spec-sync branch May 7, 2026 13:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tip-1034): sync escrow implementation with spec#3837

fix(tip-1034): sync escrow implementation with spec#3837
legion2002 merged 2 commits intotanishk/tip-1034-implfrom
tanishk/tip-1034-spec-sync

legion2002 commented May 6, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 6, 2026 •

edited

Loading

Uh oh!

tempoxyz-cyclops-bot left a comment

Uh oh!

tempoxyz-cyclops-bot May 7, 2026

Uh oh!

legion2002 May 7, 2026

Uh oh!

github-actions Bot commented May 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

legion2002 commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Changelog not found.

Uh oh!

tempoxyz-cyclops-bot left a comment

Choose a reason for hiding this comment

⚠️ [ISSUE] Channel-escrow lifecycle calls are still classified as non-payment (file outside this diff)

Uh oh!

tempoxyz-cyclops-bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

legion2002 May 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Tempo Precompiles Coverage

precompiles

contracts

Total: 5635/8492 lines (66.36%)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

legion2002 commented May 6, 2026 •

edited

Loading

github-actions Bot commented May 6, 2026 •

edited

Loading

github-actions Bot commented May 7, 2026 •

edited

Loading